Skip to content

fix(deployments): keep the filters when you come back from a deployment - #561

Open
AbdullahM07 wants to merge 1 commit into
oblien:mainfrom
AbdullahM07:fix/deployments-filter-url-state
Open

fix(deployments): keep the filters when you come back from a deployment#561
AbdullahM07 wants to merge 1 commit into
oblien:mainfrom
AbdullahM07:fix/deployments-filter-url-state

Conversation

@AbdullahM07

Copy link
Copy Markdown
Member

Reported: on 0.6.5, go to Deployments → filter by one project → click a deployment → Back → the list shows all deployments again, no longer filtered.

What was wrong

The filters were component state only:

const [filter, setFilter] = useState<...>("all")
const [searchQuery, setSearchQuery] = useState("")
const [selectedProjectId, setSelectedProjectId] = useState<string | "all">("all")

Back remounts the component, so all three reset to their defaults. Status and search were lost the same way as the project filter.

That's worse than just losing a filter: a list you had deliberately narrowed silently became the full one, and the rows on screen still looked like an answer to the question you'd asked.

The fix

The filters now live in the query string — ?status=, ?project=, ?q= — so Back returns to the URL you left and the state comes back with it. This is the same URL-as-state approach the server detail page already uses for ?tab=.

Details worth reviewing:

  • A default is absent, not ?status=all. An unfiltered visit leaves a clean address and never rewrites it.
  • The sync bails when the query string is already correct. That's the mount case, so restoring from a URL performs no navigation — and the effect can't loop against its own dependency on searchParams.
  • replace, not push. Filtering doesn't deserve history entries, and Back should leave the list rather than step back through every filter change. scroll: false so re-filtering doesn't jump to the top.
  • An unrecognised status falls back to all, so a stale or hand-edited link can't wedge the view on a value nothing matches and show an empty list.
  • Unrelated query params are preserved.
  • Search was already debounced (300ms) inside DeploymentsFilters, so typing doesn't churn the URL.

Scope

Deliberately limited to the standalone /deployments view. Embedded in a project (isProject) the list is already that project's, the project selector is hidden, and that page owns its own URL — it rewrites to /projects/:id/:tab after reading its params, which would strip anything written here and fight the sync.

Testing

7 tests mount the real component and drive the reported repro: writing each filter to the URL, restoring each from the URL on mount (which is what Back does), dropping a param when it returns to its default, rejecting an invalid status, and preserving unrelated params.

Verified they catch the regression — with the change reverted, 5 of 7 fail, including restoring the project filter.

Two testing notes:

  • Adds happy-dom. The dashboard had no DOM harness: every existing test uses renderToStaticMarkup, which runs no effects and so can never reach state that depends on a fetch or a URL read. This overlaps with fix(dashboard): stop a bad payload from making a server undeletable #560, which adds the same devDependency — whichever merges second may need a trivial lockfile rebase.
  • The row overflow menu is stubbed for one reason: it imports utils/icons, which is JSX inside a .js file that the test transform cannot parse (the monitoring suite documents the same constraint). The card itself stays real, since the card is what proves a filter was applied.

Filter the deployments list by a project, open one of the rows, press Back — and
the list returned showing every project again. Status and search did the same.

The filters were component state only, and Back remounts the component, so each
one reset to its default. A list you had deliberately narrowed silently became the
full one, which is worse than losing the filter: the rows on screen looked like an
answer to the question you had asked.

They now live in the query string (`?status=`, `?project=`, `?q=`), so Back
returns to the URL you left and the state comes back with it — the same
URL-as-state approach the server detail page already uses for `?tab=`.

Details worth knowing:

- A filter at its default is absent from the URL, not `?status=all`, so an
  unfiltered visit leaves a clean address and never rewrites it.
- The sync bails when the query string is already correct. That is the mount case,
  so restoring from a URL performs no navigation, and the effect cannot loop
  against its own dependency on searchParams.
- `replace`, not `push`: filtering does not deserve history entries, and Back
  should leave the list, not step back through filter changes. `scroll: false` so
  re-filtering does not jump to the top.
- An unrecognised status falls back to "all" rather than filtering against a value
  nothing matches, so a stale or hand-edited link cannot show an empty list.
- Unrelated query params are preserved.

Scoped to the standalone /deployments view. Embedded in a project the list is
already that project's, the project selector is hidden, and that page owns its
URL — it rewrites it to /projects/:id/:tab after reading its own params, which
would strip anything written here and fight the sync.

Tests mount the real component and drive the reported repro. Verified they catch
the regression: with the change reverted, 5 of the 7 fail, including restoring the
project filter. Adds happy-dom, needed to mount a component whose state depends on
effects — the existing renderToStaticMarkup helpers run none. The row overflow
menu is stubbed because it imports utils/icons, JSX inside a .js that the test
transform cannot parse; the card itself stays real, since the card is what proves a
filter was applied.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant